Support Custom Artifactory Repositories for Package Metadata#258
Support Custom Artifactory Repositories for Package Metadata#258voidpetal wants to merge 3 commits intoaboutcode-org:mainfrom
Conversation
voidpetal
force-pushed
the
fix-custom-artifactory-metadata
branch
2 times, most recently
from
7bae318 to
2321963
Compare
pombredanne
left a comment
pombredanne
left a comment
There was a problem hiding this comment.
Thanks. This PR really looks like it was in good part generated by a coding agent. I appreciate your efforts, but overall, do not force me to review code that you have not thoroughly reviewed yourself, otherwise you are effectively outsourcing AI generation review to me, a human, which could be considered bad form and not super polite or nice.
Time is a precious thing that is not a commodity for me. And reviewing AI slop is a waste of my time.
We are evolving our AI policy but at a high level:
- do not use AI to generate commit messages and PR bodies messages. These are used to communicate between people, so be nice and genuine: type these yourself with your own words. Here please amend your commit messages and PR body.
- adopt our style for commits messages and code. In particular for tests.
- we need doc and changelog updates
- do not add unused code. Remove it.
src/python_inspector/package_data.py
Outdated
| if repos: | ||
| # Convert to list if needed and use first repo's index_url | ||
| repos_list = list(repos) if not isinstance(repos, list) else repos | ||
| base_path = repos_list[0].index_url.replace("/simple", "/pypi") |
There was a problem hiding this comment.
What is this doing? this is weird: what if you have 10 repos?
There was a problem hiding this comment.
Good point, I initially opted out for a half-solution to get the first repository. However now I updated it to try all available repositories until it hits a success. Let me know if this aligns with your expectations.
src/python_inspector/package_data.py
Outdated
| for url_entry in response.get("urls") or []: | ||
| url = url_entry.get("url") | ||
| if url: | ||
| # Resolve relative URLs (from Artifactory) to absolute URLs |
There was a problem hiding this comment.
Do you have Artifactory examples? and what about nexus or others?
There was a problem hiding this comment.
I am not aware of any public Artifactory URL to share freely.
src/python_inspector/package_data.py
Outdated
|
|
||
| def get_file_match_key(url: str, sha256: Optional[str] = None) -> tuple: | ||
| """ | ||
| Extract a match key (filename, sha256) for comparing distribution files. |
There was a problem hiding this comment.
Use our comment style: `Return ....
There was a problem hiding this comment.
This function was unused, sorry I overlooked it..
src/python_inspector/package_data.py
Outdated
| continue | ||
|
|
||
| url_data = urls.get(dist_url) | ||
| url_data = urls_by_filename[filename] |
There was a problem hiding this comment.
Are you sure that this will not fail?
There was a problem hiding this comment.
You're right, replaced with urls_by_filename.get(filename)
voidpetal
force-pushed
the
fix-custom-artifactory-metadata
branch
3 times, most recently
from
3cafbe0 to
672f569
Compare
|
Hi @pombredanne, Thanks a lot for your review! I have ammended the PR and commit descriptions as you proposed and the ammended code is ready for your further review. Please let me know if you would like me to do anything else! |
voidpetal
force-pushed
the
fix-custom-artifactory-metadata
branch
from
672f569 to
e5ddeeb
Compare
voidpetal
changed the title
voidpetal
force-pushed
the
fix-custom-artifactory-metadata
branch
2 times, most recently
from
dce06d9 to
40dbfa4
Compare
When using --index-url with a custom Artifactory repository, dependency resolution works but the packages array comes back empty. This happens because get_pypi_data_from_purl() hardcodes https://pypi.org/pypi for the JSON API endpoint. Internal packages that don't exist on PyPI.org return 404 and are silently skipped. The fix includes deriving the JSON API base URL from the provided repository instead of hardcoding PyPI.org. It is also necessary to match distribution files by filename (standardized per PEP 427/491) instead of full URL, since URL paths can differ between Simple API and JSON API endpoints. Signed-off-by: Kai Hodžić <hodzic.e.k@outlook.com>
Signed-off-by: Kai Hodžić <hodzic.e.k@outlook.com>
Signed-off-by: Kai Hodžić <hodzic.e.k@outlook.com>
voidpetal
force-pushed
the
fix-custom-artifactory-metadata
branch
from
40dbfa4 to
9fdb092
Compare
2 participants
When using
--index-urlwith a custom Artifactory repository, dependency resolution works but thepackagesarray comes back empty. This happens becauseget_pypi_data_from_purl()hardcodeshttps://pypi.org/pypifor the JSON API endpoint. Internal packages that don't exist on PyPI.org return 404 and are silently skipped.The fix includes deriving the JSON API base URL from the provided repository instead of hardcoding PyPI.org
It is also necessary to match distribution files by filename (standardized per PEP 427/491) instead of full URL, since URL paths can differ between Simple API and JSON API endpoints.